Skip to content

Fix layout of chord brackets and parens#33356

Merged
miiizen merged 1 commit into
musescore:mainfrom
miiizen:32471-bracketParens
May 15, 2026
Merged

Fix layout of chord brackets and parens#33356
miiizen merged 1 commit into
musescore:mainfrom
miiizen:32471-bracketParens

Conversation

@miiizen
Copy link
Copy Markdown
Contributor

@miiizen miiizen commented May 12, 2026

Resolves: #32471

@miiizen miiizen requested a review from mike-spa May 12, 2026 08:30
claude[bot]
claude Bot reviewed May 12, 2026
View reviewed changes
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 12, 2026 •
edited
Loading

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1c25812e-ab63-4b04-9f86-9c4d8922c0c4

📥 Commits

Reviewing files that changed from the base of the PR and between 1f75bd1 and a7ae233.

📒 Files selected for processing (3)
  • src/engraving/rendering/score/chordlayout.cpp
  • src/engraving/rendering/score/parenthesislayout.cpp
  • vtest/scores/chordBrackets-3.mscz
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/engraving/rendering/score/chordlayout.cpp
📝 Walkthrough

Walkthrough

This PR defers chord-bracket layout out of the pitched-element loop into a dedicated pass executed after notes, fingerings, and articulations are laid out; the pitched-element loop now skips both slurs and chord brackets. It also updates ParenthesisLayout::getParentShape to exclude chord arpeggios and chord brackets (in addition to stems) when building the parent dummy shape for parentheses spacing.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is minimal but incomplete—it references the linked issue but lacks required sections and details about changes, testing, and checklist items from the template. Expand description to include motivation for changes, mark checklist items, commit messages referencing the issue, and verification of testing across commits.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main changes in the PR, which address chord bracket and parenthesis layout issues.
Linked Issues check ✅ Passed The code changes directly address the reported issue by fixing chord bracket and parenthesis layout logic to prevent horizontal expansion during editing.
Out of Scope Changes check ✅ Passed All changes are focused on chord bracket and parenthesis layout in two rendering-related files, directly supporting the linked issue's objective.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@davidstephengrant davidstephengrant added the vtests This PR produces approved changes to vtest results label May 13, 2026
@davidstephengrant
Copy link
Copy Markdown
Contributor

davidstephengrant commented May 13, 2026

@miiizen Tested and approved on Ubuntu 24.04.4 LTS.

@miiizen miiizen force-pushed the 32471-bracketParens branch from 1f75bd1 to a7ae233 Compare May 15, 2026 09:53
@miiizen miiizen merged commit f20a42f into musescore:main May 15, 2026
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@claude claude[bot] claude[bot] left review comments

@mike-spa mike-spa mike-spa approved these changes

Assignees

@davidstephengrant davidstephengrant

Labels

vtests This PR produces approved changes to vtest results

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bracket plus parenthesis can expand horizontally

3 participants

@miiizen @davidstephengrant @mike-spa